pocx: render bech32 P2WPKH addresses in user-facing account/signer messages - #2

…ssages (#2)

Operator-facing log lines and JSON-RPC errors that referenced a plot or
the effective signer printed the 20-byte hash160 as raw 40-char hex,
forcing operators to manually re-encode the value to map it back to the
pocx1q... form their wallet, get_assignment, and the rest of the chain
tooling display.

Render those identifiers via EncodeDestination(WitnessV0KeyHash{...}),
mirroring the pattern already used in get_assignment
(src/pocx/rpc/assignments.cpp). Address HRP follows the active chain, so
testnet/regtest produce tpocx1.../rpocx1... respectively.

The hex form is still computed and passed to wallet lookup APIs
(interfaces::Wallet::haveAccountKey, signPoCXBlock,
pocx::mining::HaveAccountKey), so the wallet-check contract is unchanged
- only the human-facing strings are reformatted.

Touched messages:
- src/pocx/rpc/mining.cpp (submit_nonce):
  * "Plot %s has assignment, checking key for effective signer: %s" log
  * "Wallet holding key for effective signer %s is locked..."
    JSONRPCError(RPC_WALLET_UNLOCK_NEEDED)
  * "No private key available for effective signer %s (plot: %s)"
    JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY)
- src/pocx/mining/scheduler.cpp:
  * "[Scheduler] Trying %zu wallet(s) for effective signer %s" log
- src/pocx/mining/block_builder.cpp:
  * "[BlockBuilder] Building block for account %s (...)" log
  * "[BlockBuilder] Plot: %s, Effective signer: %s at height %d" log
- src/pocx/mining/wallet_signing.cpp:
  * "Account ID: %s -> CKeyID: %s" -> "Signing for account %s"
    (the original printed the same hash twice in different reversed-hex
    forms; collapsed to a single bech32 line)
  * "No ScriptPubKeyMan found that can sign for account %s" log

Refs PoC-Consortium/bitcoin-pocx#3

Add scripts/mining/test-regtest-submitnonce-key-gate-v2.sh covering every
branch of the receive-path probe (HaveAccountKey) plus the multi-wallet
iteration semantics:

  A  Absent     watch-only descriptor; expect -5 + bech32 + no raw-hex regression
  B  Locked     encrypted+locked privkey; expect -13 + walletpassphrase hint
  C  Available  unlocked privkey; expect exit 0 + raw_quality/poc_time
  D  Multi      Locked + Available, Locked loaded first
                (probe must traverse past Locked to pick Available)
  E  Multi      Available + Locked, Available loaded first
                (later Locked must not downgrade an earlier Available)

D + E together prove the {Locked, Available} probe outcome is order-
independent, ruling out "break on first Locked" and "Locked overrides
later" regressions. Each case loads its wallets in isolation and unloads
between cases so the probe state under test is unambiguous.

Regression guards for:
  #4 (PR PoC-Consortium/bitcoin#3) - Absent/Locked split
  #3 (PR PoC-Consortium/bitcoin#2) - bech32 rendering

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… build fix

* build: bump version to v30.2.1rc1

* build: stop leaking AVX/AVX2 codegen into scalar/SSE2 TUs

batch_validation.cpp and plot_generation.cpp contain no SIMD intrinsics —
they only call into shabal256_avx2/sse2 via ordinary function calls. The
set_property(SOURCE ... COMPILE_OPTIONS ${AVX2_CXXFLAGS}) blocks therefore
served no purpose and let the compiler emit AVX/AVX2 anywhere in those
TUs (memset/memcpy expansion to YMM, VEX-encoded SSE, auto-vectorization).
The result was SIGILL on non-AVX2 x86-64 CPUs before the runtime
HaveAVX2() gate could fire.

Drop the three set_property blocks; keep target_compile_definitions
ENABLE_AVX2, which is just a #define and has no codegen impact. The
intrinsics-bearing TUs (shabal256_avx2.cpp, sha256_avx2.cpp) keep their
flags via src/crypto/CMakeLists.txt and are unaffected.

Fixes PoC-Consortium/bitcoin-pocx#1.

* wallet: fail create/revoke_assignment when fee estimation falls back and -fallbackfee is disabled

CreateForgingTransactionImpl resolved the feerate via GetMinimumFeeRate
with feeCalc=nullptr, so it could not see when the result came from the
fallback path. When smart-fee estimation has no data and -fallbackfee=0
(Core's default since 0.20), GetMinimumFeeRate returns CFeeRate(0) and
skips the required-feerate floor. The builder then produced a 0-fee tx
that the wallet committed locally but the mempool rejected with "min
relay fee not met", leaving an unbroadcastable tx and a spent input.

Mirror the guard from src/wallet/spend.cpp: pass a FeeCalculation, and
return an error when reason==FALLBACK and !m_allow_fallback_fee. Same
message as Core's normal spend path. Single fix point covers both
create_assignment and revoke_assignment (shared impl).

Also fix two clang-tidy bugprone-argument-comment errors at the Coin
construction site by aligning the /*height=*/, /*coinbase=*/ comment
names with the actual parameter names nHeightIn / fCoinBaseIn.

Fixes PoC-Consortium/bitcoin-pocx#2.

* pocx: render bech32 P2WPKH addresses in user-facing account/signer messages (#2)

Operator-facing log lines and JSON-RPC errors that referenced a plot or
the effective signer printed the 20-byte hash160 as raw 40-char hex,
forcing operators to manually re-encode the value to map it back to the
pocx1q... form their wallet, get_assignment, and the rest of the chain
tooling display.

Render those identifiers via EncodeDestination(WitnessV0KeyHash{...}),
mirroring the pattern already used in get_assignment
(src/pocx/rpc/assignments.cpp). Address HRP follows the active chain, so
testnet/regtest produce tpocx1.../rpocx1... respectively.

The hex form is still computed and passed to wallet lookup APIs
(interfaces::Wallet::haveAccountKey, signPoCXBlock,
pocx::mining::HaveAccountKey), so the wallet-check contract is unchanged
- only the human-facing strings are reformatted.

Touched messages:
- src/pocx/rpc/mining.cpp (submit_nonce):
  * "Plot %s has assignment, checking key for effective signer: %s" log
  * "Wallet holding key for effective signer %s is locked..."
    JSONRPCError(RPC_WALLET_UNLOCK_NEEDED)
  * "No private key available for effective signer %s (plot: %s)"
    JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY)
- src/pocx/mining/scheduler.cpp:
  * "[Scheduler] Trying %zu wallet(s) for effective signer %s" log
- src/pocx/mining/block_builder.cpp:
  * "[BlockBuilder] Building block for account %s (...)" log
  * "[BlockBuilder] Plot: %s, Effective signer: %s at height %d" log
- src/pocx/mining/wallet_signing.cpp:
  * "Account ID: %s -> CKeyID: %s" -> "Signing for account %s"
    (the original printed the same hash twice in different reversed-hex
    forms; collapsed to a single bech32 line)
  * "No ScriptPubKeyMan found that can sign for account %s" log

Refs PoC-Consortium/bitcoin-pocx#3

* pocx: gate nonce submissions on actual privkey availability (#3)

* pocx: gate nonce submissions on actual privkey availability, not script membership

The receive-path check in submit_nonce ultimately resolved to
DescriptorScriptPubKeyMan::IsMine(script), which is pure script-map
membership. Wallets loaded with pubkey-only descriptors (importdescriptors
with an xpub, watch-only setups) passed the gate and the scheduler then
built unsigned blocks that signing dropped on the floor seconds later.

Replace HaveAccountKey's isSpendable probe with the same predicate the
signer actually executes: iterate ScriptPubKeyMans, CanProvide as a cheap
script-only pre-filter, then GetPoCXPubKey under cs_wallet to verify a
private key is loadable. CWallet::SignMessage is the established Core
precedent for both the loop shape and the lock placement (cs_wallet ->
cs_desc_man order, see comment at wallet/wallet.cpp:2215).

Return AccountKeyAvailability instead of bool so the locked-vs-absent
triage in pocx/rpc/mining.cpp keeps producing RPC_WALLET_UNLOCK_NEEDED
for encrypted-and-locked wallets instead of regressing them to the
generic "no private key" error. SignPoCXBlock adopts the same idiom for
consistency and to close the matching latent deadlock window. Misleading
"failed to get public key" log line corrected to mention the actual
cause.

Refs PoC-Consortium/bitcoin-pocx#4

* pocx: simplify submit_nonce dispatch and document forward-decl

Drop the dead has_key flag from submit_nonce and replace the if/else if/else
chain with two unconditional throws (Locked, then Absent); Available falls
through. Same semantics, no intermediate state to track.

Add a one-line note on the AccountKeyAvailability forward declaration in
interfaces/wallet.h pointing at the underlying-type definition in
pocx/mining/wallet_signing.h, since the two must stay in sync for the
forward decl to remain valid.

* wallet: expose pocx_type field in transaction RPC responses (#5)

Add an optional pocx_type field to listtransactions, gettransaction, and
listsinceblock that is set to "assignment" or "revocation" when the
transaction carries the corresponding PoCX OP_RETURN marker, and is
absent otherwise.

The field is pushed inside WalletTxToJSON — the shared helper reached by
all three RPCs (directly by gettransaction, and via ListTransactions
with fLong=true for the other two) — and declared once in
TransactionDescriptionString so the RPCResult documentation flows to
every affected RPC. Marker detection short-circuits on the first hit;
the assignment / revocation OP_RETURN parsers in
pocx/assignments/opcodes.h are reused as-is.

Both the include and the field are guarded with #ifdef ENABLE_POCX,
matching the convention used elsewhere in src/wallet/.

Closes PoC-Consortium/bitcoin-pocx#6.

* pocx: bring assignment DB to chainstate-grade crash resilience (#4)

* pocx: bring assignment DB to chainstate-grade crash resilience

Assignment writes used to flush in a separate LevelDB WriteBatch issued
after CCoinsViewDB::BatchWrite had already committed DB_BEST_BLOCK. A
process kill between the two batches (Windows Update reboots being the
common trigger) left the node with BEST_BLOCK advanced past the assignment
updates from that flush window. Validation then consulted stale assignment
state, rejected valid blocks, and the node got isolated. The existing
HEAD_BLOCKS / ReplayBlocks recovery only rebuilt coins, so the corruption
was permanent.

Fold the assignment writes into the chainstate's final batch and extend
the replay path to rebuild assignments alongside coins:

- CCoinsView::BatchWrite (and Backed / Cache / DB overrides) gains optional
  ForgingAssignmentsMap and DeletedAssignmentsSet parameters under
  ENABLE_POCX. CCoinsViewCache::Flush()/Sync() build the maps as before
  and pass them through one BatchWrite call. CCoinsViewDB::BatchWrite
  appends WriteAssignmentsToBatch to the final CDBBatch right before
  DB_BEST_BLOCK is written, so assignment updates are atomic with the
  chainstate transition and bracketed by DB_HEAD_BLOCKS.

- BatchWriteAssignments is removed from the CCoinsView interface (and
  all implementations) — the only callers were the two flush paths.
  WriteAssignmentsToBatch becomes a private helper on CCoinsViewDB.

- New CCoinsViewCache::LookupForgingAssignmentForReplay consults
  pendingAssignments first and falls back to base, so revocation lookups
  during ReplayBlocks can find assignments added earlier in the same
  replay window before any flush has reached the DB.

- New helper pocx::assignments::ApplyAssignmentEffectsForReplay mirrors
  the OP_RETURN extraction in ConnectBlock without consensus checks.
  Chainstate::RollforwardBlock calls it per tx so HEAD_BLOCKS replay
  reconstructs assignment state idempotently from block data.

- Test view subclasses in src/test/coins_tests.cpp and
  src/test/fuzz/coinscache_sim.cpp are updated to match the new virtual
  signature.

No new on-disk schema, no fsync changes, no read cache, no new recovery
state machine — the assignment DB now rides the same two-phase commit
+ replay pattern Bitcoin Core already uses for coins.


* pocx: move ApplyAssignmentEffectsForReplay out of consensus lib

Place the replay-side helper in a new pocx/assignments/replay.{h,cpp}
pair compiled into bitcoin_node, since it mutates CCoinsViewCache
(common lib) and uses LogPrintf (util lib) — neither of which the
consensus library is allowed to depend on. opcodes.{h,cpp} stays in
bitcoin_consensus as pure script/parsing helpers.

Fixes contrib/devtools/check-deps.sh failures for
CCoinsViewCache::{Add,Update,LookupForgingAssignmentForReplay} and
LogInstance / BCLog::Logger::LogPrintStr imported by opcodes.cpp.o.


* pocx: fix RemoveForgingAssignment leak across reorg disconnects

RemoveForgingAssignment short-circuited after clearing a matching entry
from pendingAssignments, assuming "found in pending" implied "never
flushed to DB." That assumption breaks during a reorg: when
DisconnectTip replays a revocation, RestoreForgingAssignment seeds the
pre-revocation assignment into pendingAssignments, but the original
add may already have been flushed to LevelDB on an earlier periodic
flush. The subsequent DisconnectTip of the add finds the txid in
pending, removes it, and returns — leaving the DB row intact.

After the reorg the node carries a stale assignment row that no fresh
peer on the new chain has. Symptoms include forging-acceptance and
assignment-rejection divergence at heights where the node thinks the
plot is ASSIGNED but peers see UNASSIGNED, and the stale row survives
restart (no DB_HEAD_BLOCKS marker, no ReplayBlocks trigger).

Fix: drop the early return so both removal paths run. Removing from
pending and queueing a DB delete are idempotent, and only one or the
other will actually find a match unless the reorg pattern above is in
play, in which case both must run. Suppress the "not found" log when
the pending path succeeded so the common case stays quiet.


* pocx: make assignment cache layer symmetric with its writes

c66a10ec91 folded the assignment writes into the chainstate's single
BatchWrite (good — fixes the crash bug) and removed the cascading
BatchWriteAssignments that used to write through to LevelDB on every
intermediate cache flush. The read path was left querying base
directly, so any state between block-connect and the chainstate's next
flush became invisible: get_assignment reported UNASSIGNED for plots
that had just been assigned, and the disconnect-side
RemoveForgingAssignment couldn't find rows that lived in the parent
cache's pendingAssignments. Every regtest assignment test except the
crash-revive one started failing.

The architectural gap was that the cache layer cached its writes but
didn't read from the cache. Fix it the same way Bitcoin Core handles
coins: pending/deleted shadow base on reads, and the parent's
BatchWrite keeps pending and deleted mutually exclusive on writes.

- GetForgingAssignmentHistory: new CCoinsViewCache override that drops
  base rows queued for deletion in this window and overlays pending
  entries (upsert by txid).
- GetForgingAssignment: derives from the merged history — walks pending
  in reverse for the most recent entry whose assignment_height is at
  or before the requested height, falling back to base.
- CCoinsViewCache::BatchWrite: when a child flushes, skip pending
  insert for keys also in deletedAssignmentsIn and remove any matching
  pending row before recording the deletion. Upsert pending rows by
  txid so revocations / replays don't accumulate duplicates.

All 10 regtest assignment scripts now pass on this branch (baseline:
9/10, PR head before this fix: 3/10).


* pocx: cancel queued deletion when same key is re-added to cache

When the same tx ends up in competing blocks during a reorg, the cache
needs to keep pending and deletedAssignments mutually exclusive for any
(plot, txid). The earlier disconnect of the losing block queues the DB
row for deletion in deletedAssignments; the subsequent connect of the
winning block adds the assignment back to pending via
AddForgingAssignment / UpdateForgingAssignment / RestoreForgingAssignment
or via BatchWrite from a child cache.

Without clearing the deletion intent, the next flush emits both a Write
and an Erase for the same DB key in a single LevelDB batch — Erase wins
and the assignment row disappears even though the canonical chain still
asserts it. The cache itself is wiped by the same flush, so the node is
left blind to its own consensus state.

Clear the matching deletedAssignments entry at every entry point that
re-adds to pending so the invariant holds end-to-end.


* pocx: tighten WriteAssignmentsToBatch — skip dead writes, O(log N) deletes

Two pre-existing efficiency wins, surfaced while reviewing the cache
path:

- The write loop emitted a Write for every entry in `assignments` even
  when the same key was about to be erased in the deletion loop. The
  cache hands the deletion payload alongside its key in `assignments`
  so the DB can derive the height-indexed row to erase, so a Write +
  Erase pair on the same key was unavoidable previously. Skip the
  Write when `deletedAssignments` contains the key — Erase still runs
  with the height from the payload.

- The deletion loop reconstructed the height by std::find_if-ing the
  whole `assignments` map per deleted key (O(N·M)). The map is keyed
  by (plot, txid), so a direct .find() drops it to O(N·log M).

No behavior change beyond the eliminated dead Writes.


* pocx: count newly-deleted assignments in cachedAssignmentsUsage

RemoveForgingAssignment already increments cachedAssignmentsUsage when
it adds a row to deletedAssignments, but BatchWrite's deletion path did
not — so each child-driven deletion silently dropped the cache-size
estimate by sizeof(ForgingAssignment). Pre-existing nit; doesn't affect
correctness, only the cache-pressure heuristic that decides when to
flush.

Use try_emplace so a re-deletion of the same key (payload refresh) does
not double-count.


* pocx: drop LookupForgingAssignmentForReplay, refresh stale cache comments

LookupForgingAssignmentForReplay returned pendingAssignments.back() or
fell back to base->GetForgingAssignment(plot, MAX_INT). Since the cache's
GetForgingAssignment is now merge-aware (pending overlays base, deletions
suppress matching base rows), GetForgingAssignment(plot, MAX_INT) covers
the same use case with the same semantics — replay processes blocks in
height order, so pending.back() == argmax(assignment_height) in pending.

Also refresh the typedef and member comments that still described
pendingAssignments / deletedAssignments as if the cache only used them
for duplicate detection / reorg undo. With the merge-aware reads they
are the cache's primary read overlay.

No behaviour change beyond the eliminated call hop.


* pocx: tighten ENABLE_POCX gating in coins/txdb without rewriting upstream lines

The earlier integration replaced original upstream signatures and
Flush()/Sync() call sites with POCX-aware versions, deleting the
original code instead of extending it. Restore the upstream lines and
gate only the POCX-specific bits inline:

- BatchWrite signatures in CCoinsView, CCoinsViewBacked, CCoinsViewCache,
  and CCoinsViewDB now keep `const uint256 &hashBlock` (upstream style)
  and add the POCX parameters via a single inline `#ifdef ENABLE_POCX`
  block.
- CCoinsViewCache::Flush() and ::Sync() keep one `base->BatchWrite(cursor,
  hashBlock ... )` call site; only the extra POCX arguments and the
  prep / cleanup blocks are gated. No more separate non-POCX `bool fOk`
  line that diverges from upstream.
- Drop the stale `<limits>` include in coins.cpp (its only user was the
  removed LookupForgingAssignmentForReplay).

Refactor only — behaviour unchanged. Full regtest assignment suite
(11/11) still passes.


* pocx: revert coinscache_sim.cpp signature tweak — dead code

src/test/fuzz/CMakeLists.txt replaces the fuzz target with an empty
stub when ENABLE_POCX is ON, so coinscache_sim.cpp is never compiled
in POCX builds. In POCX-off builds the #ifdef ENABLE_POCX branch we
added is also skipped. The override-signature extension reached no
compilation path; revert to upstream.


* pocx: separate PoCX comments from upstream comments, collapse txdb.h gating

coins.h: the CCoinsView::BatchWrite docstring is upstream Bitcoin Core
documentation — restore it verbatim and put the PoCX-specific note
inside the ENABLE_POCX gate as a separate `//!` line next to the
PoCX parameters. Also restore the upstream `const uint256& hashBlock`
spelling on the virtual decl.

txdb.h: CCoinsViewDB had two ENABLE_POCX blocks with the upstream
StoragePath() sandwiched between them. Move StoragePath() up next to
the other non-PoCX public methods so the PoCX additions sit in a
single trailing block (public assignment getters → private
WriteAssignmentsToBatch helper, all inside one #ifdef).

Refactor only — no behavioural change. Full regtest assignment suite
(11/11) still passes.

* build: bump version to v30.2.1rc3

* build: bump version to v30.2.1

… corruption check in fees.dat

fa1d17d refactor: Use uint64_t over size_t for serialize corruption check in fees.dat (MarcoFalke)

Pull request description:

  Serialization should not behave differently on different architectures. See also the related commit 3789215.

  However, on fees.dat file corruption, 32-bit builds may run into an unsigned integer overflow and report the wrong corruption reason, or may even silently continue after the corruption.

  This is a bit hard to reproduce, because 32-bit platforms are rare and most of them don't support running the unsigned integer overflow sanitizer. So the possible options to reproduce are:

  * Run on armhf and manually annotate the code to detect the overflow
  * Run on i386 with the integer sanitizer (possibly via `podman run -it --rm --platform linux/i386 'debian:trixie'`)
  * Run the integer sanitizer on any 64-bit platform and manually replace type in the affected line by `uint32_t`

  Afterwards, the steps to reproduce are:

  ```
  export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git  --depth=1 ./b-c && cd b-c && apt install build-essential cmake pkg-config  python3-zmq libzmq3-dev libevent-dev libboost-dev libsqlite3-dev  systemtap-sdt-dev  libcapnp-dev capnproto  libqrencode-dev qt6-tools-dev qt6-l10n-tools qt6-base-dev  clang llvm libc++-dev libc++abi-dev   -y

  cmake -B ./bld-cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DSANITIZERS=undefined,integer,float-divide-by-zero --preset=dev-mode

  cmake --build ./bld-cmake --parallel  $(nproc)

  curl -fLO 'https://github.com/bitcoin-core/qa-assets/raw/b5ad78e070e4cf36beb415d7b490d948d70ba73f/fuzz_corpora/policy_estimator_io/607473137013139e3676e30ec4b29639e673fa9b'

  UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=policy_estimator_io ./bld-cmake/bin/fuzz ./607473137013139e3676e30ec4b29639e673fa9b
  ```

  The output will be something like:

  ```
  /b-c/src/policy/fees/block_policy_estimator.cpp:448:25: runtime error: unsigned integer overflow: 346685954 * 219 cannot be represented in type 'unsigned int'
      #0 0x5b0b1bbe in TxConfirmStats::Read(AutoFile&, unsigned int) /b-c/bld-cmake/src/./policy/fees/block_policy_estimator.cpp:448:25
      #1 0x5b0b7d3f in CBlockPolicyEstimator::Read(AutoFile&) /b-c/bld-cmake/src/./policy/fees/block_policy_estimator.cpp:1037:29
      #2 0x592a9783 in policy_estimator_io_fuzz_target(std::span<unsigned char const, 4294967295u>) /b-c/bld-cmake/src/test/fuzz/./test/fuzz/policy_estimator_io.cpp:32:32
      #3 0x5896ba8e in void std::__invoke_impl<void, void (*&)(std::span<unsigned char const, 4294967295u>), std::span<unsigned char const, 4294967295u>>(std::__invoke_other, void (*&)(std::span<unsigned char const, 4294967295u>), std::span<unsigned char const, 4294967295u>&&) /usr/lib/gcc/i686-linux-gnu/14/../../../../include/c++/14/bits/invoke.h:61:14
      #4 0x5896b8eb in std::enable_if<is_invocable_r_v<void, void (*&)(std::span<unsigned char const, 4294967295u>), std::span<unsigned char const, 4294967295u>>, void>::type std::__invoke_r<void, void (*&)(std::span<unsigned char const, 4294967295u>), std::span<unsigned char const, 4294967295u>>(void (*&)(std::span<unsigned char const, 4294967295u>), std::span<unsigned char const, 4294967295u>&&) /usr/lib/gcc/i686-linux-gnu/14/../../../../include/c++/14/bits/invoke.h:111:2
      #5 0x5896b44b in std::_Function_handler<void (std::span<unsigned char const, 4294967295u>), void (*)(std::span<unsigned char const, 4294967295u>)>::_M_invoke(std::_Any_data const&, std::span<unsigned char const, 4294967295u>&&) /usr/lib/gcc/i686-linux-gnu/14/../../../../include/c++/14/bits/std_function.h:290:9
      #6 0x59845c95 in std::function<void (std::span<unsigned char const, 4294967295u>)>::operator()(std::span<unsigned char const, 4294967295u>) const /usr/lib/gcc/i686-linux-gnu/14/../../../../include/c++/14/bits/std_function.h:591:9
      #7 0x5983a0da in test_one_input(std::span<unsigned char const, 4294967295u>) /b-c/bld-cmake/src/test/fuzz/util/./test/fuzz/fuzz.cpp:88:5
      #8 0x5983cb80 in main /b-c/bld-cmake/src/test/fuzz/util/./test/fuzz/fuzz.cpp:271:13
      #9 0xf75aecc2  (/lib/i386-linux-gnu/libc.so.6+0x24cc2) (BuildId: 2dc5f2945fad35c1b07d1a5a32520b3c41afaa75)
      #10 0xf75aed87 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x24d87) (BuildId: 2dc5f2945fad35c1b07d1a5a32520b3c41afaa75)
      #11 0x58932db6 in _start (/b-c/bld-cmake/bin/fuzz+0x235ddb6) (BuildId: 7d8d83a77923f14e99c0de64acbc5f5bfc2cce9b)

  SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /b-c/src/policy/fees/block_policy_estimator.cpp:448:25
  ```

  Note: This is marked a "refactor", because the code change does not affect 64-bit builds, and on the still remaining rare 32-bit builds today it is extremely unlikely to happen in production.

ACKs for top commit:
  bensig:
    ACK fa1d17d
  ismaelsadeeq:
    utACK fa1d17d
  luke-jr:
    Also, utACK fa1d17d as an improvement.

Tree-SHA512: 696bf8e0dbe4777c84cb90e313c7f8f9ee90d4b3e64de1222f8472b2d9d0f3a0f6f027fda743dd6ca8c6aab94f404db7a65bb562a76000d9c33a8a39de28d8d4

…ntracts

3281824 fuzz: prevent invalid `FRESH` entries and surface `BatchWrite` errors (Lőrinc)
780f460 fuzz: avoid invalid `AddCoin` overwrites (Lőrinc)
d7e0d51 fuzz: make `AddCoins` query view for overwrites (Lőrinc)
b8fa6f0 util: introduce `TrySub` to prevent unsigned underflow (Lőrinc)

Pull request description:

  ### Problem
  This is an alternative approach to bitcoin#34647, fixes bitcoin#34645.

  ### Fix
  First, add `CheckedSub` and use it for decrements of `m_dirty_count` and `cachedCoinsUsage`, so unsigned underflows turn into immediate failures instead of silently wrapping and only failing later.

  <details><summary>Assertion `j <= i' failed.</summary>

  ```bash
  util/overflow.h:44 T CheckedSub(const T, const U) [T = unsigned long, U = bool]: Assertion `j <= i' failed.
  ==72817== ERROR: libFuzzer: deadly signal
      #0 0x556e9225eab5 in __sanitizer_print_stack_trace (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x191dab5) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
      #1 0x556e921acafc in fuzzer::PrintStackTrace() (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x186bafc) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
      #2 0x556e92191bb7 in fuzzer::Fuzzer::CrashCallback() (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x1850bb7) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
      #3 0x7164cfc458cf  (/lib/x86_64-linux-gnu/libc.so.6+0x458cf) (BuildId: ae7440bbdce614e0e79280c3b2e45b1df44e639c)
      #4 0x7164cfca49bb in __pthread_kill_implementation nptl/pthread_kill.c:43:17
      #5 0x7164cfca49bb in __pthread_kill_internal nptl/pthread_kill.c:89:10
      #6 0x7164cfca49bb in pthread_kill nptl/pthread_kill.c:100:10
      #7 0x7164cfc4579d in raise signal/../sysdeps/posix/raise.c:26:13
      #8 0x7164cfc288cc in abort stdlib/abort.c:73:3
      #9 0x556e92f9d591 in assertion_fail(std::source_location const&, std::basic_string_view<char, std::char_traits<char>>) /mnt/my_storage/bitcoin/src/util/check.cpp:41:5
      #10 0x556e9250daf0 in bool&& inline_assertion_check<false, bool>(bool&&, std::source_location const&, std::basic_string_view<char, std::char_traits<char>>) /mnt/my_storage/bitcoin/src/util/check.h:90:13
      #11 0x556e9250daf0 in unsigned long CheckedSub<unsigned long, bool>(unsigned long, bool) /mnt/my_storage/bitcoin/src/util/overflow.h:44:5
      #12 0x556e9250daf0 in CoinsViewCacheCursor::NextAndMaybeErase(std::pair<COutPoint const, CCoinsCacheEntry>&) /mnt/my_storage/bitcoin/src/coins.h:282:25
      bitcoin#13 0x556e92507eb2 in (anonymous namespace)::MutationGuardCoinsViewCache::BatchWrite(CoinsViewCacheCursor&, uint256 const&) /mnt/my_storage/bitcoin/src/test/fuzz/coins_view.cpp:90:75
      bitcoin#14 0x556e92c17a2b in CCoinsViewCache::Flush(bool) /mnt/my_storage/bitcoin/src/coins.cpp:282:11
      bitcoin#15 0x556e924fb732 in TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_1::operator()() const /mnt/my_storage/bitcoin/src/test/fuzz/coins_view.cpp:135:34
      bitcoin#16 0x556e924fb732 in unsigned long CallOneOf<TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_0, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_1, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_2, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_3, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_4, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_5, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_6, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_7, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_8, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_9, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_10, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_11>(FuzzedDataProvider&, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_0, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_1, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_2, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_3, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_4, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_5, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_6, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_7, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_8, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_9, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_10, TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool)::$_11) /mnt/my_storage/bitcoin/src/test/fuzz/util.h:42:27
      bitcoin#17 0x556e924fb732 in TestCoinsView(FuzzedDataProvider&, CCoinsViewCache&, CCoinsView&, bool) /mnt/my_storage/bitcoin/src/test/fuzz/coins_view.cpp:114:9
      bitcoin#18 0x556e92503b0c in coins_view_overlay_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>) /mnt/my_storage/bitcoin/src/test/fuzz/coins_view.cpp:404:5
      bitcoin#19 0x556e92bcb7a5 in std::function<void (std::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::span<unsigned char const, 18446744073709551615ul>) const /usr/lib/gcc/x86_64-linux-gnu/15/../../../../include/c++/15/bits/std_function.h:593:9
      bitcoin#20 0x556e92bcb7a5 in test_one_input(std::span<unsigned char const, 18446744073709551615ul>) /mnt/my_storage/bitcoin/src/test/fuzz/fuzz.cpp:88:5
      bitcoin#21 0x556e92bcb7a5 in LLVMFuzzerTestOneInput /mnt/my_storage/bitcoin/src/test/fuzz/fuzz.cpp:216:5
      bitcoin#22 0x556e9219318f in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x185218f) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
      bitcoin#23 0x556e92192799 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x1851799) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
      bitcoin#24 0x556e92194139 in fuzzer::Fuzzer::MutateAndTestOne() (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x1853139) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
      bitcoin#25 0x556e92194c95 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x1853c95) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
      bitcoin#26 0x556e92181255 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x1840255) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
      bitcoin#27 0x556e921ad696 in main (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x186c696) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)
      bitcoin#28 0x7164cfc2a577 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
      bitcoin#29 0x7164cfc2a63a in __libc_start_main csu/../csu/libc-start.c:360:3
      bitcoin#30 0x556e921757e4 in _start (/mnt/my_storage/bitcoin/build_fuzz/bin/fuzz+0x18347e4) (BuildId: d77c4d5f9dfd38ea06fab463f49341735205e109)

  NOTE: libFuzzer has rudimentary signal handlers.
        Combine libFuzzer with AddressSanitizer or similar for better crash reports.
  SUMMARY: libFuzzer: deadly signal
  MS: 2 PersAutoDict-CopyPart- DE: "\005\000"-; base unit: ecb626aff8724f0fdde38a0a6965718f2096d474
  artifact_prefix='/tmp/fuzz_artifacts/'; Test unit written to /tmp/fuzz_artifacts/crash-1d19026c1a23f08bfe693fd684a56ce51187c6e5
  ./build_fuzz/bin/fuzz /tmp/fuzz_corpus/coins_view_overlay -max_total_time=3600 -rss_limit_mb=2560 -artifact_prefix=/tmp/fuzz_artifacts/ >fuzz-16.log 2>&1
  ```

  </details>

  The coins view fuzz targets can call `AddCoin`/`AddCoins` and construct `BatchWrite` cursors in ways that violate `CCoinsViewCache` caller contracts. These invalid states can trigger `BatchWrite` `std::logic_error` and can desync dirty-entry accounting (caught by `Assume(m_dirty_count == 0)` currently).

  Make the fuzzer avoid generating invalid states instead of catching and resetting:
  * Derive `AddCoin`’s `possible_overwrite` from `PeekCoin`, so `possible_overwrite=false` is only used when the outpoint is absent - similarly to https://github.com/bitcoin/bitcoin/blob/67c0d1798e6147f48d4bafc2c9e5ff30f2a62340/src/test/fuzz/coinscache_sim.cpp#L312-L317
  - Only use `AddCoins(check=false)` when we have confirmed the txid has no unspent outputs; otherwise fall back to `check=true` so `AddCoins` determines overwrites via the view.
  - When constructing a `CoinsViewCacheCursor`, avoid setting `FRESH` when the parent already has an unspent coin, and ensure `FRESH` implies `DIRTY`.

  ### Fuzzing
  The original error could be reproduced in ~10 minutes using `coins_view_overlay`. I ran the `coins_view`, `coins_view_db`, `coins_view_overlay`, and `coinscache_sim` fuzzers for this PR overnight and they didn't fail anymore.

ACKs for top commit:
  achow101:
    ACK 3281824
  sipa:
    ACK 3281824. Ran the 4 relevant fuzz tests for ~1 CPU day each. Will run more overnight.
  andrewtoth:
    ACK 3281824

Tree-SHA512: b8155e8d21740eb7800e373c27a8a1457eb84468c24af879bac5a1ed251ade2aec99c34a350a31f2ebb74e41bb7380bf20214d38d14fe23310a43282d2434fb7

…Sync bench

fa79098 test: Fix shutdown vptr race in BlockFilterIndexSync bench (MarcoFalke)

Pull request description:

  Currently, the `BlockFilterIndexSync` may fail tsan.

  Diff to reproduce:

  ```diff
  diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
  index c7be6ab..4cb8435 100644
  --- a/src/validationinterface.cpp
  +++ b/src/validationinterface.cpp
  @@ -14,2 +14,3 @@
   #include <primitives/transaction.h>
  +#include <random.h>
   #include <util/check.h>
  @@ -156,2 +157,4 @@ void ValidationSignals::SyncWithValidationInterfaceQueue()

  +static FastRandomContext g_rnd{};
  +
   // Use a macro instead of a function for conditional logging to prevent
  @@ -166,2 +169,3 @@ void ValidationSignals::SyncWithValidationInterfaceQueue()
               LOG_EVENT(fmt, local_name, __VA_ARGS__);           \
  +            UninterruptibleSleep(1ms * g_rnd.randrange(95));   \
               event();                                           \
  ```

  and then running the tsan CI pod: `MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_native_tsan.sh" ./ci/test_run_all.sh`

  After about 3 runs, it will fail. It is also possible to run in a loop inside the pod: `while TSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/tsan:halt_on_error=1:second_deadlock_stack=1" /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/bin/bench_bitcoin -filter=BlockFilterIndexSync -sanity-check ; do true ; done`

  The output will be:

  ```
  Running with -sanity-check option, output is being suppressed as benchmark results will be useless.
  Running with -sanity-check option, output is being suppressed as benchmark results will be useless.
  ==================
  WARNING: ThreadSanitizer: data race on vptr (ctor/dtor vs virtual call) (pid=100168)
    Write of size 8 at 0x7fffbe828aa8 by main thread:
      #0 BaseIndex::~BaseIndex() /ci_container_base/src/index/base.cpp:99:1 (bench_bitcoin+0x33c201) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #1 BlockFilterIndex::~BlockFilterIndex() /ci_container_base/src/index/blockfilterindex.h:40:7 (bench_bitcoin+0x266000) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #2 BlockFilterIndexSync(ankerl::nanobench::Bench&)::$_0::operator()() const /ci_container_base/src/bench/index_blockfilter.cpp:56:5 (bench_bitcoin+0x2659a1) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #3 ankerl::nanobench::Bench& ankerl::nanobench::Bench::run<BlockFilterIndexSync(ankerl::nanobench::Bench&)::$_0>(BlockFilterIndexSync(ankerl::nanobench::Bench&)::$_0&&) /ci_container_base/src/bench/nanobench.h:1221:13 (bench_bitcoin+0x2659a1)
      #4 BlockFilterIndexSync(ankerl::nanobench::Bench&) /ci_container_base/src/bench/index_blockfilter.cpp:46:33 (bench_bitcoin+0x26565a) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #5 std::__1::__invoke_result_impl<void, void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&>::type std::__1::__invoke[abi:dee230000]<void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&>(void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&) /cxx_build/include/c++/v1/__type_traits/invoke.h:90:27 (bench_bitcoin+0x21b394) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #6 void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:dee230000]<void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&>(void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&) /cxx_build/include/c++/v1/__type_traits/invoke.h:350:5 (bench_bitcoin+0x21b394)
      #7 void std::__1::__invoke_r[abi:dee230000]<void, void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&>(void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&) /cxx_build/include/c++/v1/__type_traits/invoke.h:356:10 (bench_bitcoin+0x21b394)
      #8 std::__1::__function::__func<void (*)(ankerl::nanobench::Bench&), void (ankerl::nanobench::Bench&)>::operator()(ankerl::nanobench::Bench&) /cxx_build/include/c++/v1/__functional/function.h:172:12 (bench_bitcoin+0x21b394)
      #9 std::__1::__function::__value_func<void (ankerl::nanobench::Bench&)>::operator()[abi:dee230000](ankerl::nanobench::Bench&) const /cxx_build/include/c++/v1/__functional/function.h:273:12 (bench_bitcoin+0x1cc77d) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #10 std::__1::function<void (ankerl::nanobench::Bench&)>::operator()(ankerl::nanobench::Bench&) const /cxx_build/include/c++/v1/__functional/function.h:754:10 (bench_bitcoin+0x1cc77d)
      #11 benchmark::BenchRunner::RunAll(benchmark::Args const&) /ci_container_base/src/bench/bench.cpp:121:13 (bench_bitcoin+0x1cc77d)
      #12 main /ci_container_base/src/bench/bench_bitcoin.cpp:135:9 (bench_bitcoin+0x1c5a76) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)

    Previous read of size 8 at 0x7fffbe828aa8 by thread T1:
      #0 ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_1::operator()() const::'lambda'(CValidationInterface&)::operator()(CValidationInterface&) const /ci_container_base/src/validationinterface.cpp:231:79 (bench_bitcoin+0x885feb) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #1 void ValidationSignalsImpl::Iterate<ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_1::operator()() const::'lambda'(CValidationInterface&)>(ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_1::operator()() const::'lambda'(CValidationInterface&)&&) /ci_container_base/src/validationinterface.cpp:91:17 (bench_bitcoin+0x885feb)
      #2 ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_1::operator()() const /ci_container_base/src/validationinterface.cpp:231:22 (bench_bitcoin+0x885feb)
      #3 ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_0::operator()() const /ci_container_base/src/validationinterface.cpp:233:27 (bench_bitcoin+0x885feb)
      #4 std::__1::__invoke_result_impl<void, ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_0&>::type std::__1::__invoke[abi:dee230000]<ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_0&>(ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_0&) /cxx_build/include/c++/v1/__type_traits/invoke.h:90:27 (bench_bitcoin+0x885feb)
      #5 void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:dee230000]<ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_0&>(ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_0&) /cxx_build/include/c++/v1/__type_traits/invoke.h:350:5 (bench_bitcoin+0x885feb)
      #6 void std::__1::__invoke_r[abi:dee230000]<void, ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_0&>(ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_0&) /cxx_build/include/c++/v1/__type_traits/invoke.h:356:10 (bench_bitcoin+0x885feb)
      #7 std::__1::__function::__func<ValidationSignals::MempoolTransactionsRemovedForBlock(std::__1::vector<RemovedMempoolTransactionInfo, std::__1::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int)::$_0, void ()>::operator()() /cxx_build/include/c++/v1/__functional/function.h:172:12 (bench_bitcoin+0x885feb)
      #8 std::__1::__function::__value_func<void ()>::operator()[abi:dee230000]() const /cxx_build/include/c++/v1/__functional/function.h:273:12 (bench_bitcoin+0xddec83) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #9 std::__1::function<void ()>::operator()() const /cxx_build/include/c++/v1/__functional/function.h:754:10 (bench_bitcoin+0xddec83)
      #10 SerialTaskRunner::ProcessQueue() /ci_container_base/src/scheduler.cpp:173:5 (bench_bitcoin+0xddec83)
      #11 SerialTaskRunner::MaybeScheduleProcessQueue()::$_0::operator()() const /ci_container_base/src/scheduler.cpp:142:41 (bench_bitcoin+0xde08e5) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #12 std::__1::__invoke_result_impl<void, SerialTaskRunner::MaybeScheduleProcessQueue()::$_0&>::type std::__1::__invoke[abi:dee230000]<SerialTaskRunner::MaybeScheduleProcessQueue()::$_0&>(SerialTaskRunner::MaybeScheduleProcessQueue()::$_0&) /cxx_build/include/c++/v1/__type_traits/invoke.h:90:27 (bench_bitcoin+0xde08e5)
      bitcoin#13 void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:dee230000]<SerialTaskRunner::MaybeScheduleProcessQueue()::$_0&>(SerialTaskRunner::MaybeScheduleProcessQueue()::$_0&) /cxx_build/include/c++/v1/__type_traits/invoke.h:350:5 (bench_bitcoin+0xde08e5)
      bitcoin#14 void std::__1::__invoke_r[abi:dee230000]<void, SerialTaskRunner::MaybeScheduleProcessQueue()::$_0&>(SerialTaskRunner::MaybeScheduleProcessQueue()::$_0&) /cxx_build/include/c++/v1/__type_traits/invoke.h:356:10 (bench_bitcoin+0xde08e5)
      bitcoin#15 std::__1::__function::__func<SerialTaskRunner::MaybeScheduleProcessQueue()::$_0, void ()>::operator()() /cxx_build/include/c++/v1/__functional/function.h:172:12 (bench_bitcoin+0xde08e5)
      bitcoin#16 std::__1::__function::__value_func<void ()>::operator()[abi:dee230000]() const /cxx_build/include/c++/v1/__functional/function.h:273:12 (bench_bitcoin+0xddda36) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      bitcoin#17 std::__1::function<void ()>::operator()() const /cxx_build/include/c++/v1/__functional/function.h:754:10 (bench_bitcoin+0xddda36)
      bitcoin#18 CScheduler::serviceQueue() /ci_container_base/src/scheduler.cpp:60:17 (bench_bitcoin+0xddda36)
      bitcoin#19 ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2::operator()() const /ci_container_base/src/test/util/setup_common.cpp:250:114 (bench_bitcoin+0x2dcaa8) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      bitcoin#20 std::__1::__invoke_result_impl<void, ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2&>::type std::__1::__invoke[abi:dee230000]<ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2&>(ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2&) /cxx_build/include/c++/v1/__type_traits/invoke.h:90:27 (bench_bitcoin+0x2dcaa8)
      bitcoin#21 void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:dee230000]<ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2&>(ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2&) /cxx_build/include/c++/v1/__type_traits/invoke.h:350:5 (bench_bitcoin+0x2dcaa8)
      bitcoin#22 void std::__1::__invoke_r[abi:dee230000]<void, ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2&>(ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2&) /cxx_build/include/c++/v1/__type_traits/invoke.h:356:10 (bench_bitcoin+0x2dcaa8)
      bitcoin#23 std::__1::__function::__func<ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2, void ()>::operator()() /cxx_build/include/c++/v1/__functional/function.h:172:12 (bench_bitcoin+0x2dcaa8)
      bitcoin#24 std::__1::__function::__value_func<void ()>::operator()[abi:dee230000]() const /cxx_build/include/c++/v1/__functional/function.h:273:12 (bench_bitcoin+0xef2b0b) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      bitcoin#25 std::__1::function<void ()>::operator()() const /cxx_build/include/c++/v1/__functional/function.h:754:10 (bench_bitcoin+0xef2b0b)
      bitcoin#26 util::TraceThread(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>) /ci_container_base/src/util/thread.cpp:21:9 (bench_bitcoin+0xef2b0b)
      bitcoin#27 std::__1::__invoke_result_impl<void, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), char const*, ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2>::type std::__1::__invoke[abi:dee230000]<void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), char const*, ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2>(void (*&&)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), char const*&&, ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2&&) /cxx_build/include/c++/v1/__type_traits/invoke.h:90:27 (bench_bitcoin+0x2dc652) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      bitcoin#28 void std::__1::__thread_execute[abi:dee230000]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), char const*, ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2, 0ul, 1ul, 2ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), char const*, ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2>&, std::__1::__integer_sequence<unsigned long, 0ul, 1ul, 2ul>) /cxx_build/include/c++/v1/__thread/thread.h:161:3 (bench_bitcoin+0x2dc652)
      bitcoin#29 void* std::__1::__thread_proxy[abi:dee230000]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), char const*, ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2>>(void*) /cxx_build/include/c++/v1/__thread/thread.h:169:3 (bench_bitcoin+0x2dc652)

    Location is stack of main thread.

    Thread T1 'b-scheduler' (tid=100170, running) created by main thread at:
      #0 pthread_create <null> (bench_bitcoin+0x13dc4e) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #1 std::__1::__libcpp_thread_create[abi:dee230000](unsigned long*, void* (*)(void*), void*) /cxx_build/include/c++/v1/__thread/support/pthread.h:182:10 (bench_bitcoin+0x2d3531) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #2 std::__1::thread::thread[abi:dee230000]<void (&)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), char const (&) [10], ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2, 0>(void (&)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), char const (&) [10], ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts)::$_2&&) /cxx_build/include/c++/v1/__thread/thread.h:218:16 (bench_bitcoin+0x2d3531)
      #3 ChainTestingSetup::ChainTestingSetup(ChainType, TestOpts) /ci_container_base/src/test/util/setup_common.cpp:250:46 (bench_bitcoin+0x2d3531)
      #4 TestingSetup::TestingSetup(ChainType, TestOpts) /ci_container_base/src/test/util/setup_common.cpp:344:7 (bench_bitcoin+0x2d4c1f) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #5 TestChain100Setup::TestChain100Setup(ChainType, TestOpts) /ci_container_base/src/test/util/setup_common.cpp:380:7 (bench_bitcoin+0x2d560a) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #6 std::__1::unique_ptr<TestChain100Setup, std::__1::default_delete<TestChain100Setup>> std::__1::make_unique[abi:dee230000]<TestChain100Setup, ChainType const&, TestOpts&, 0>(ChainType const&, TestOpts&) /cxx_build/include/c++/v1/__memory/unique_ptr.h:756:30 (bench_bitcoin+0x224eed) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #7 std::__1::unique_ptr<TestChain100Setup, std::__1::default_delete<TestChain100Setup>> MakeNoLogFileContext<TestChain100Setup>(ChainType, TestOpts) /ci_container_base/src/test/util/setup_common.h:259:12 (bench_bitcoin+0x224ce2) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #8 BlockFilterIndexSync(ankerl::nanobench::Bench&) /ci_container_base/src/bench/index_blockfilter.cpp:33:29 (bench_bitcoin+0x26530f) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #9 std::__1::__invoke_result_impl<void, void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&>::type std::__1::__invoke[abi:dee230000]<void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&>(void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&) /cxx_build/include/c++/v1/__type_traits/invoke.h:90:27 (bench_bitcoin+0x21b394) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      #10 void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:dee230000]<void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&>(void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&) /cxx_build/include/c++/v1/__type_traits/invoke.h:350:5 (bench_bitcoin+0x21b394)
      #11 void std::__1::__invoke_r[abi:dee230000]<void, void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&>(void (*&)(ankerl::nanobench::Bench&), ankerl::nanobench::Bench&) /cxx_build/include/c++/v1/__type_traits/invoke.h:356:10 (bench_bitcoin+0x21b394)
      #12 std::__1::__function::__func<void (*)(ankerl::nanobench::Bench&), void (ankerl::nanobench::Bench&)>::operator()(ankerl::nanobench::Bench&) /cxx_build/include/c++/v1/__functional/function.h:172:12 (bench_bitcoin+0x21b394)
      bitcoin#13 std::__1::__function::__value_func<void (ankerl::nanobench::Bench&)>::operator()[abi:dee230000](ankerl::nanobench::Bench&) const /cxx_build/include/c++/v1/__functional/function.h:273:12 (bench_bitcoin+0x1cc77d) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)
      bitcoin#14 std::__1::function<void (ankerl::nanobench::Bench&)>::operator()(ankerl::nanobench::Bench&) const /cxx_build/include/c++/v1/__functional/function.h:754:10 (bench_bitcoin+0x1cc77d)
      bitcoin#15 benchmark::BenchRunner::RunAll(benchmark::Args const&) /ci_container_base/src/bench/bench.cpp:121:13 (bench_bitcoin+0x1cc77d)
      bitcoin#16 main /ci_container_base/src/bench/bench_bitcoin.cpp:135:9 (bench_bitcoin+0x1c5a76) (BuildId: d6021b6fabe0b72ef5741922cc8e7f71ce6eac0c)

  SUMMARY: ThreadSanitizer: data race on vptr (ctor/dtor vs virtual call) /ci_container_base/src/index/base.cpp:99:1 in BaseIndex::~BaseIndex()
  ==================
  ```

  Fix this by following the shutdown sequence of first stopping the index and then desctructing it, instead of doing both at the same time (stopping inside the desctructor).

  Also, apply the comment `// Shutdown sequence (c.f. Shutdown() in init.cpp)` consistently, while touching this topic of the codebase.

  Also, remove the unused `SyncWithValidationInterfaceQueue`, which is redundant to the prior `BlockUntilSyncedToCurrentChain`. See also the last comment in the pull request that introduced this: bitcoin#26188 (comment)

  > I think anything is fine here. Either keep both or delete both.

  Given that devs did not apply the redundant sync in two new cases, it seems fine to remove it.

ACKs for top commit:
  achow101:
    ACK fa79098
  sedited:
    ACK fa79098

Tree-SHA512: 3f46df283fa5f639e942b74459760104b79f11930b4249f73405f052131f69d01e91a8a4df496ca4f380fd2e0cb8d8c34f70f3e2b84730f3b8f445a98e468d1d